fix(core): honor the gitignore \# escape when loading ignore patterns - #1540
wangzhengzhuo05 wants to merge 1 commit into
Conversation
.bmignore documents gitignore-style syntax, but the loader skipped any line beginning with '#' and never unescaped a leading backslash, so no pattern beginning with a hash could take effect: '#*#' was dropped as a comment and '\#*#' was stored literally and never matched. Strip one leading backslash when a line starts with '\#', for both .bmignore and project .gitignore, so e.g. '\#*#' loads as '#*#' (Emacs autosave files). Fixes basicmachines-co#1539 Signed-off-by: wangzhengzhuo05 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbffd1a117
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| line = raw_line.strip() | ||
| if not line or line.startswith("#"): | ||
| return None | ||
| if line.startswith("\\#"): | ||
| return line[1:] |
There was a problem hiding this comment.
When a .gitignore contains \#foo, a valid pattern for the filename #foo, strip() removes the significant leading space and the new escape branch converts the remainder to #foo. Basic Memory therefore ignores/index-excludes the wrong file, while git check-ignore --no-index matches only #foo; Git's pattern format discards unescaped trailing spaces, not leading ones. Preserve leading characters when removing the line ending before recognizing the hash escape.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in replacement #1544, commit ee2f854. The parser preserves leading whitespace, decodes escaped hashes/spaces, and discards only unescaped trailing spaces. Regressions cover the reported space-plus-escaped-hash case for both loaders: 39 focused tests pass, all 22 filename expectations agree with git check-ignore, and just fast-check plus just doctor pass. Exact-head Codex gate is currently waiting on #1544.
|
Thanks @wangzhengzhuo05 for the fix and regressions. The replacement is now open for review at #1544, based on current main. It preserves your original commit and author/DCO attribution (rebased without changing its patch), with a separate maintainer commit for the whitespace correction. The Codex P2 is addressed in Replacement #1544 is in exact-head CI/Codex review now (Codex gate: waiting). Nothing has been merged; this original PR remains open while the replacement completes review. |
|
Superseded by #1544 (merged), which preserves this patch's commit and authorship. Closing to keep the review queue clean — thanks @phernandez for carrying it forward with the whitespace correction. |
2 participants
What
.bmignore(and project.gitignore) could not express any pattern beginning with#.Why
Both loaders used a naive line filter that skipped every line starting with
#and neverunescaped a leading backslash:
#*#was silently dropped as a comment.\#*#was stored literally, andshould_ignore_path()matches withfnmatch, whichhas no escape syntax — so it matched nothing.
The failure mode is silent and misleading: the pattern looks right in
.bmignoreandsimply never loads. It makes Emacs autosave files (
#file#) impossible to ignore throughthe documented gitignore-style syntax, even though the shipped defaults already cover
*~,*.swpand*.swo. As the issue notes, the matcher itself handles#*#correctly(it
fnmatches each path part) — only the loader was broken.How
Added one small module-level helper,
_parse_ignore_pattern_line(), and routed both the.bmignoreand.gitignoreread loops through it:#) are still skipped;\#now yields the pattern with one leading backslash stripped,per the gitignore rule "Put a backslash in front of the first hash for patterns that
begin with a hash" — so
\#*#loads as#*#and matches.should_ignore_path()is unchanged,DEFAULT_IGNORE_PATTERNSis unchanged (adding adefault
#*#would be a separate product decision), and no dependency was added(no
pathspec).Tests
Three regression tests in
tests/cli/test_ignore_utils.py:test_bmignore_escaped_hash_pattern_loads—\#*#in.bmignoreloads as#*#, thecomment line is still skipped, and
should_ignore_path("notes/#note.md#")isTrue.test_bmignore_bare_hash_is_still_a_comment— a bare#*#remains a comment.test_gitignore_escaped_hash_pattern_loads— same escape handling for a project.gitignore.Mutation check
Reverting only the escape handling in
_parse_ignore_pattern_line()(the\#branchremoved, everything else intact) fails exactly the two tests that guard the fix:
Restoring the fix →
17 passed.Fixes #1539